-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove Cli #1444
Remove Cli #1444
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1444 +/- ##
==========================================
- Coverage 89.78% 87.42% -2.36%
==========================================
Files 161 139 -22
Lines 5972 5178 -794
==========================================
- Hits 5362 4527 -835
- Misses 610 651 +41
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on if there are any commands left in starfish CLI (e.g., starfish validate
), you may be able to dump the custom click package as well. I suspect there is value to keeping starfish validate
though.
@@ -57,9 +57,6 @@ jobs: | |||
- name: 3D smFISH data processing example | |||
if: type = push and branch =~ /^(master|merge)/ | |||
script: make install-dev 3d_smFISH.py | |||
- name: iss_cli.sh data processing example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can plausibly remove the Makefile rules for running .sh
data processing examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed!
from starfish.core.image.Filter.reduce import Reduce | ||
|
||
|
||
methods: Mapping[str, Type] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you just do a dir(Filter)
to get this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried this and it doesn't quite give me what I need. ['Bandpass', 'Clip', 'ClipPercentileToZero', 'ClipValueToZero', 'DeconvolvePSF', 'ElementWiseMultiply', 'GaussianHighPass', 'GaussianLowPass', 'Laplace', 'LinearUnmixing', 'MatchHistograms', 'MaxProject', 'MeanHighPass', 'Reduce', 'WhiteTophat', 'ZeroByChannelMagnitude', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '_base', 'bandpass', 'clip', 'clip_percentile_to_zero', 'clip_value_to_zero', 'element_wise_mult', 'gaussian_high_pass', 'gaussian_low_pass', 'laplace', 'linear_unmixing', 'match_histograms', 'max_proj', 'mean_high_pass', 'reduce', 'richardson_lucy_deconvolution', 'util', 'white_tophat', 'zero_by_channel_magnitude']
but I think the refactor to make another clever auto import method will work when I do that!
@@ -75,20 +75,6 @@ origin. At this point, it's trivial to create a cell x gene matrix. | |||
|
|||
tutorials/exec_feature_identification_and_annotation.rst | |||
|
|||
Putting Together a Pipeline Recipe and Running it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we dropping the recipe runner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took out all the recipe stuff :/ should leave the runner in?
|
||
:: | ||
|
||
$ starfish validate experiment tmp/experiment.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to preserve some documentation about starfish validate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still have just the validate documentation in but I took out the builder bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is plausible that we want to retain the example about validating an existing experiment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls see comments.
@@ -57,9 +57,6 @@ jobs: | |||
- name: 3D smFISH data processing example | |||
if: type = push and branch =~ /^(master|merge)/ | |||
script: make install-dev 3d_smFISH.py | |||
- name: iss_cli.sh data processing example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping
|
||
:: | ||
|
||
$ starfish validate experiment tmp/experiment.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is plausible that we want to retain the example about validating an existing experiment.
from starfish.core.image.Filter.reduce import Reduce | ||
|
||
|
||
methods: Mapping[str, Type] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping
Co-Authored-By: Tony Tung <ttung@chanzuckerberg.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks really nice ....
This broke the docker test. |
Remove CLI code and pipeline component class.
Rename package names so that the algorithms can be used in the same way, ex.
Changed AlgorithmBase to be the MetaClass for logging.
Everything api-wise should work the same. Everything cli-wise should be removed.
One exception was the starfish validate command. Also I kept the ascii art cause that's essential